Skip to content

[GEN][ZH] Fix VTune GameDataDebug.ini field parse error introduced by #1231 #1245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 9, 2025
Merged

[GEN][ZH] Fix VTune GameDataDebug.ini field parse error introduced by #1231 #1245

merged 1 commit into from
Jul 9, 2025

Conversation

xezon
Copy link

@xezon xezon commented Jul 8, 2025

This change fixes the VTune GameDataDebug.ini field parse error introduced by #1231.

@xezon xezon added Blocker Severity: Minor < Major < Critical < Blocker Bug Something is not working right, typically is user facing Debug Is mostly debug functionality ThisProject The issue was introduced by this project, or this task is specific to this project labels Jul 8, 2025
@xezon xezon mentioned this pull request Jul 8, 2025
Closed
aliendroid1
aliendroid1 approved these changes Jul 9, 2025
View reviewed changes
Copy link

@aliendroid1 aliendroid1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this alone fixes all issues that were introduced but as far as I can tell, all the the changes made here are correct

Mauller
Mauller approved these changes Jul 9, 2025
View reviewed changes
Copy link

@Mauller Mauller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should prevent the issue, in my own i only fixed the places which were causing problems without reintroducing vTune as a thing into the debug build.

@aliendroid1
Copy link

This should prevent the issue, in my own i only fixed the places which were causing problems without reintroducing vTune as a thing into the debug build.

that was obviously better. We should completely remove vtune in a future pr

@xezon
Copy link
Author

xezon commented Jul 9, 2025

Coincidentally this is how I had the VTune originally but then changed it during the review process.

@aliendroid1
Copy link

Coincidentally this is how I had the VTune originally but then changed it during the review process.

yeah but you didn't do what the reviewers asked. You just decided that moving it to profile fixed the issue and marked it as resolved. Even now you're still not removing/disabling the bad code.

I gave this an approving review because I think not as bad this way and can still be removed in a future pr whilst making the build work again in the mean time

@xezon
Copy link
Author

xezon commented Jul 9, 2025

The other change was for removing RTS_INTERNAL. Not removing VTune.

@xezon
Copy link
Author

xezon commented Jul 9, 2025

Tested in Generals and Zero Hour with RTS_DEBUG. Worked.

@xezon xezon merged commit 321ff11 into TheSuperHackers:main Jul 9, 2025
14 checks passed
@xezon xezon deleted the xezon/fix-vtune-debug branch July 9, 2025 16:19
@aliendroid1
Copy link

The other change was for removing RTS_INTERNAL. Not removing VTune.

Yes but it was also not supposed to be for moving vtune into debug

Caball009 pushed a commit to Caball009/GeneralsGameCode that referenced this pull request Jul 10, 2025
@xezon @Caball009
[GEN][ZH] Fix VTune GameDataDebug.ini field parse error introduced by T…
a632526 
…heSuperHackers#1231 (TheSuperHackers#1245)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mauller Mauller Mauller approved these changes

@aliendroid1 aliendroid1 aliendroid1 approved these changes

Assignees
No one assigned
Labels
Blocker Severity: Minor < Major < Critical < Blocker Bug Something is not working right, typically is user facing Debug Is mostly debug functionality ThisProject The issue was introduced by this project, or this task is specific to this project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xezon @aliendroid1 @Mauller